Don't reveal credentials in log and error output - #232
Merged
Merged
Conversation
Added tests to confirm the solution. Changed how credentials are parsed to avoid leaking passwords on unquoted passwords with multiple @ signs. Cleanup, unification and refactoring of credentials masking. Update of readme to include information about new behavior.
|
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #172
Summary
The password of the connect string could end up on screen in two ways:
Argsdebug line. At startup the CLI logs its command line arguments at debug level, connect string included:Passwords containing
@. The connect-string parser ended an unquoted password at the first@. Forapp/p@ss@host, the password was taken aspand the connect string asss@host. The connection failed, and part of the password appeared in the connection lines and in the driver's error message, printed on every run:Behaviour change
Passwords containing @ no longer have to be quoted, thought it's a good ide to keep it that way. Connect strings can not contain
@because everything up to the last@is taken as the password.This is fine as connect strings (EZConnect, TNS aliases, descriptors) don't normally contain
@.